The early return seems abrupt. It also loses your intention. Why not just say if( !IsSubDevice){ urDeviceRetain } then it's clear WHY we are tracking isSubDevice .

Otherwise, someone in the future could easily refactor this code and the retain could end up higher in the function and the issue reintroduced.

Done

please implement a helper function that taken an IsSubDevice bool to eliminate duplication.

Done

The destructor device_impl::~device_impl calls urDeviceRelease, which is meant to balance the urDeviceRetain in the constructor. However, after this change, urDeviceRetain is no longer called for subdevices in the constructor, while urDeviceRelease is still invoked for them in the destructor. This imbalance is somewhat concerning. It seems that the actual root cause of the memory leak may lie elsewhere. Could you please clarify what you believe the underlying issue is?

The reason is that when a device is created RefCount is equal 1 without any call to retain(), but it is destroyed when RefCount is equal 0, so there should be one more call to release() than a number of calls to retain() ... or the implementation of class RefCount is wrong ?

class RefCount {
public:
  RefCount(uint32_t count = 1) : Count(count) {}
  ...
  uint32_t retain() { return ++Count; }
  bool release() { return --Count == 0; }
  void reset(uint32_t value = 1) { Count = value; }

private:
  std::atomic_uint32_t Count;
};

...

ur_result_t urDeviceRelease(ur_device_handle_t Device) {
  // Root devices are destroyed during the piTearDown process.
  if (Device->isSubDevice()) {
    if (Device->RefCount.release()) {
      delete Device;
    }
  }

  return UR_RESULT_SUCCESS;
}

Also worth pointing out that only subdevices are really ref counted:

https://github.com/oneapi-src/unified-runtime/blob/cc3c318865438aceba8a967f238fa151994ad811/source/adapters/level_zero/device.cpp#L1737

@ldorau @pbalcer In such case, why we differentiate between root device and sub-device in this case? can we just remove retain from the constructor completely? (only sub-devices being ref counted seems like implementation detail of the adapter)

"(only sub-devices being ref counted seems like implementation detail of the adapter)"

That's the reason. I pointed it out because it's possible that the existing sequence of creates/retains/releases was always broken, and we never noticed because the leak happens in the rare case of subdevices. So in a way, I'm agreeing with your earlier comment.